Skip to content

Fix ransack_alias issue, close #1239 - #1512

Merged
scarroll32 merged 6 commits into
activerecord-hackery:mainfrom
itsalongstory:issue_1239
Sep 24, 2025
Merged

Fix ransack_alias issue, close #1239#1512
scarroll32 merged 6 commits into
activerecord-hackery:mainfrom
itsalongstory:issue_1239

Conversation

@itsalongstory

@itsalongstory itsalongstory commented Aug 1, 2024

Copy link
Copy Markdown
Contributor

Close #1239

test_ransack.rb

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'
  gem 'activerecord', '~> 7.1', '>= 7.1.3.4', require: "active_record"
  gem 'sqlite3', '~> 1.7', '>= 1.7.3'
  gem 'ransack', '~> 4.2'
  gem 'minitest', '~> 5.24', '>= 5.24.1', require: "minitest/autorun"
end

ActiveRecord::Base.establish_connection(
  adapter:  "sqlite3",
  database: "./test_ransack_alias"
)

ActiveRecord::Schema.define do
  drop_table(:fees, if_exists: true)

  create_table :fees do |t|
    t.integer :amount
  end
end


class Fee < ActiveRecord::Base
  ransack_alias :amount_a, :amount

  def self.ransackable_attributes(auth_object = nil)
    ["amount", "amount_a"]
  end
end

class MyTest < Minitest::Test
  # success
  def test_1
    query_params = { amount_not_eq: 1 }
    assert_equal "SELECT \"fees\".* FROM \"fees\" WHERE \"fees\".\"amount\" != 1", Fee.ransack(query_params).result.to_sql
  end

  # success
  def test_2
    query_params = { amount_not_eq: 1, amount_a_not_eq: 2 }
    assert_equal "SELECT \"fees\".* FROM \"fees\" WHERE (\"fees\".\"amount\" != 1 AND \"fees\".\"amount\" != 2)", Fee.ransack(query_params).result.to_sql
  end

  # failure
  def test_3
    query_params = { amount_a_not_eq: 2, amount_not_eq: 1 }
    assert_equal "SELECT \"fees\".* FROM \"fees\" WHERE (\"fees\".\"amount\" != 2 AND \"fees\".\"amount\" != 1)", Fee.ransack(query_params).result.to_sql
  end
end
@itsalongstory ➜ /workspaces $ ruby test_ransack.rb 
Fetching gem metadata from https://rubygems.org/........
Resolving dependencies...
-- drop_table(:fees, {:if_exists=>true})
   -> 0.0272s
-- create_table(:fees)
   -> 0.0005s
Run options: --seed 4070

# Running:

F..

Finished in 0.010956s, 273.8346 runs/s, 273.8346 assertions/s.

  1) Failure:
MyTest#test_3 [test_ransack.rb:49]:
--- expected
+++ actual
@@ -1 +1 @@
-"SELECT \"fees\".* FROM \"fees\" WHERE (\"fees\".\"amount\" != 2 AND \"fees\".\"amount\" != 1)"
+"SELECT \"fees\".* FROM \"fees\" WHERE \"fees\".\"amount\" != 1"


3 runs, 3 assertions, 1 failures, 0 errors, 0 skips

@le0pard

le0pard commented Sep 13, 2024

Copy link
Copy Markdown

Can #1525 also fix this issue? Because now reject! based on name and key in condition

@itsalongstory

Copy link
Copy Markdown
Contributor Author

Can #1525 also fix this issue? Because now reject! based on name and key in condition

#1525 doesn't fix this.

@le0pard

le0pard commented Sep 14, 2024

Copy link
Copy Markdown

@itsalongstory thanks for check

@itsalongstory

Copy link
Copy Markdown
Contributor Author

Can #1525 also fix this issue? Because now reject! based on name and key in condition

diff --git a/lib/ransack/nodes/grouping.rb b/lib/ransack/nodes/grouping.rb
index 95d6488..6d72097 100644
--- a/lib/ransack/nodes/grouping.rb
+++ b/lib/ransack/nodes/grouping.rb
@@ -53,7 +53,7 @@ module Ransack
       end
 
       def []=(key, value)
-        conditions.reject! { |c| c.same_name_or_key?(key) }
+        conditions.reject! { |c| c.same_name_or_key?(key) && c.value == value.value }
         self.conditions << value
       end

Maybe we need to reject! based on name and key, and also value.

@le0pard

le0pard commented Sep 14, 2024

Copy link
Copy Markdown

ok, got a problem. Maybe this commit will be enough - aa2062c

@scarroll32
scarroll32 requested a review from Copilot September 24, 2025 16:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes an issue with ransack_alias where conditions with the same aliased attribute name but different values were incorrectly being overwritten instead of being combined as separate conditions.

  • Modifies the condition rejection logic to preserve conditions with different values
  • Ensures that aliased attributes with different values generate multiple WHERE clauses

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread lib/ransack/nodes/grouping.rb Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@scarroll32
scarroll32 requested a review from Copilot September 24, 2025 16:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@scarroll32 scarroll32 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot can you please add test cases to handle the various nil cases to ensure the reject! is working in all cases?

    conditions.reject! { |c| c.key == key.to_s && c&.value == value&.value }

@scarroll32
scarroll32 enabled auto-merge (squash) September 24, 2025 19:40
@scarroll32
scarroll32 merged commit 9336f5a into activerecord-hackery:main Sep 24, 2025
24 checks passed
@itsalongstory
itsalongstory deleted the issue_1239 branch February 13, 2026 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ransack_alias sometimes does not work correctly

5 participants